Use the RN packager's host by default by shilman · Pull Request #1568 · storybookjs/storybook

shilman · 2017-08-01T18:16:41Z

What I did

Use the RN packager's host by default as described in #1222

added CRNA example for testing
updated the CLI templates

How to test

First:

yarn && yarn bootstrap

Then test RN:

yarn bootstrap:react-native-vanilla
cd examples/react-native-vanilla
yarn storybook
./node_modules/.bin/react-native run-ios

Then test CNRA:

yarn bootstrap:crna-kitchen-sink
cd examples/crna-kitchen-sink
yarn storybook
yarn ios

oblador · 2017-08-01T18:53:57Z

@@ -0,0 +1,25 @@
+export default from './storybook';
+
+// import React from 'react';


Is this left here intentionally?

Yeah, I'll add to the comment tho. 😄

oblador · 2017-08-01T18:56:23Z

 const StorybookUI = getStorybookUI({
  port: 7007,
-  host: 'localhost',
+  // host: 'localhost',


Should probably be deleted.

oblador · 2017-08-01T18:57:25Z

+  };
+
+  showApp(event) {
+    event.preventDefault();


Forgive my ignorance, but why is preventDefault needed?

copypasta? i'm not sure.

I'm also unsureFWIW

It's all over the code (part of the CLI boilerplate), so I propose we address it separately 😕

You do not typically need to .preventDefault() in React Native. That said I don't see the showApp function used anywhere in this component... so couldn't this method just be removed?

although - this might be something CRNA related so if so, please ignore my thoughts on removing it.

OK OK looks like it isn't used so I'll remove across the board, test, and submit. Thanks 🙇

On third thought, I think this is a separate issue. This used to be used to show off addon-links but was removed at some point. Opened: #1569

codecov · 2017-08-01T19:07:51Z

Codecov Report

Merging #1568 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1568      +/-   ##
==========================================
- Coverage   20.43%   20.43%   -0.01%     
==========================================
  Files         241      241              
  Lines        5250     5252       +2     
  Branches      656      645      -11     
==========================================
  Hits         1073     1073              
- Misses       3661     3682      +21     
+ Partials      516      497      -19

Impacted Files	Coverage Δ
app/react-native/src/preview/index.js	`0% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/configs/handle_routing.js	`28.94% <0%> (ø)`	⬆️
app/react-native/src/bin/storybook-start.js	`0% <0%> (ø)`	⬆️
addons/notes/src/react.js	`0% <0%> (ø)`	⬆️
addons/knobs/src/KnobManager.js	`35.41% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/routes.js	`0% <0%> (ø)`	⬆️
app/react/src/client/preview/element_check.js	`41.17% <0%> (ø)`	⬆️
app/react/src/client/preview/error_display.js	`0% <0%> (ø)`	⬆️
addons/info/src/components/markdown/htags.js	`30% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/libs/filters.js	`53.33% <0%> (ø)`	⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cdc25a...67a7093. Read the comment docs.

orta

I ran the same code in our non-CRNA app, and everything was good:

import { NativeModules } from "react-native"
console.log(NativeModules.SourceCode.scriptURL)

And got http://localhost:8081/Example/Emission/index.ios.bundle?platform=ios&dev=true&hot=true

So this is fine for non-CRNA too 👍

atticoos · 2017-08-01T19:41:11Z

      if (params.resetStorybook || !channel) {
-        const host = params.host || 'localhost';
-
+        const host = params.host || parse(NativeModules.SourceCode.scriptURL).hostname;


Brilliant - didn’t know this was an accessible thing

atticoos · 2017-08-01T19:43:09Z

-        const host = params.host || 'localhost';
-
+        const host = params.host || parse(NativeModules.SourceCode.scriptURL).hostname;
        const port = params.port !== false ? `:${params.port || 7007}` : '';


Is this the port for the ReactNative packager? Or where storybooks web previewed will be binding to?

this is the storybook port

shilman added 2 commits August 1, 2017 11:12

Add CRNA kitchen sink example for testin

aebecbe

Use RN packager host by default in examples and also CLI templates

8f10fce

shilman changed the title ~~1222 parse rn packager host~~ Use the RN packager's host by default Aug 1, 2017

shilman requested review from atticoos, orta, rmevans9 and tmeasday August 1, 2017 18:17

shilman mentioned this pull request Aug 1, 2017

Stories doesn't show up in the browser #1398

Closed

oblador reviewed Aug 1, 2017

View reviewed changes

Updated RN packager host with PR comments

279dc89

shilman added feature request react-native labels Aug 1, 2017

orta approved these changes Aug 1, 2017

View reviewed changes

shilman mentioned this pull request Aug 1, 2017

Add addon-link example to react-native #1569

Closed

3 tasks

Merge branch 'master' into 1222-parse-rn-packager-host

67a7093

shilman merged commit d7bc10d into master Aug 1, 2017

shilman deleted the 1222-parse-rn-packager-host branch August 1, 2017 19:41

atticoos reviewed Aug 1, 2017

View reviewed changes

D1no mentioned this pull request Aug 28, 2017

RN: Unable to resolve module AccessibilityInfo #1503

Closed

fjtrujy mentioned this pull request Jan 31, 2019

Bundled Android application is not connecting to Web Socket server #5435

Closed

		@@ -0,0 +1,25 @@
		export default from './storybook';

		// import React from 'react';

Uh oh!

Conversation

shilman commented Aug 1, 2017

What I did

How to test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shilman Aug 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

orta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shilman Aug 1, 2017 •

edited

Loading

codecov bot commented Aug 1, 2017 •

edited

Loading